-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: shuffle code and responsibility of LocalState #49
Conversation
Achieve this by moving the state saving and loading responsibility to the LocalState itself
This reverts commit 913e3e9. # Conflicts: # linkup-cli/src/remote_local.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff!
let server_config = ServerConfig::from(&state); | ||
|
||
let server_session_name = load_config( | ||
&state.linkup.remote, | ||
&state.linkup.session_name, | ||
remote_server_conf, | ||
server_config.remote, | ||
)?; | ||
let local_session_name = load_config(&local_url, &server_session_name, local_server_conf)?; | ||
let local_session_name = load_config(&local_url, &server_session_name, server_config.local)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok 👍 Cool!
linkup-cli/src/background_booting.rs
Outdated
.map(|local_service| StorableService { | ||
name: local_service.name.clone(), | ||
location: if local_service.current == ServiceTarget::Remote { | ||
local_service.remote.clone() | ||
} else { | ||
local_service.local.clone() | ||
}, | ||
rewrites: Some(local_service.rewrites.clone()), | ||
}) | ||
.collect::<Vec<StorableService>>(); | ||
|
||
let remote_server_services = state | ||
.services | ||
.iter() | ||
.map(|local_service| StorableService { | ||
name: local_service.name.clone(), | ||
location: if local_service.current == ServiceTarget::Remote { | ||
local_service.remote.clone() | ||
} else { | ||
state.linkup.tunnel.clone() | ||
}, | ||
rewrites: Some(local_service.rewrites.clone()), | ||
}) | ||
.collect::<Vec<StorableService>>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: the local_service
variable name in the map here is confusing, maybe just service?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or switch local to remote in the remote case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point! Changed on 28055f6
Description
This changes make so that we stop using methods from the
start
command around the code.To achieve this, the responsibility of loading and saving the state is now part of the implementation
of the
LocalState
itself.Other notable changes
StorableSession
, now there is aServerConfig
whichholds a
local
andremote
fields. Where this struct is located could be moved, but didn't gave that much thought.